Skip to content

Conversation

@CoTinker
Copy link
Contributor

This PR erases spirv.mlir.loop with an empty region in LoopPattern, resolving a crash. Fixes #113404.

This PR erases `spirv.mlir.loop` with an empty region in `LoopPattern`, resolving a crash.
@llvmbot
Copy link
Member

llvmbot commented Oct 24, 2024

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-spirv

Author: Longsheng Mou (CoTinker)

Changes

This PR erases spirv.mlir.loop with an empty region in LoopPattern, resolving a crash. Fixes #113404.


Full diff: https://github.com/llvm/llvm-project/pull/113527.diff

2 Files Affected:

  • (modified) mlir/lib/Conversion/SPIRVToLLVM/SPIRVToLLVM.cpp (+6)
  • (modified) mlir/test/Conversion/SPIRVToLLVM/control-flow-ops-to-llvm.mlir (+8)
diff --git a/mlir/lib/Conversion/SPIRVToLLVM/SPIRVToLLVM.cpp b/mlir/lib/Conversion/SPIRVToLLVM/SPIRVToLLVM.cpp
index 74c169c9a7e76a..044a3c8c65e3b2 100644
--- a/mlir/lib/Conversion/SPIRVToLLVM/SPIRVToLLVM.cpp
+++ b/mlir/lib/Conversion/SPIRVToLLVM/SPIRVToLLVM.cpp
@@ -1083,6 +1083,12 @@ class LoopPattern : public SPIRVToLLVMConversion<spirv::LoopOp> {
     if (loopOp.getLoopControl() != spirv::LoopControl::None)
       return failure();
 
+    // `spirv.mlir.loop` with empty region is redundant and should be erased.
+    if (loopOp.getBody().empty()) {
+      rewriter.eraseOp(loopOp);
+      return success();
+    }
+
     Location loc = loopOp.getLoc();
 
     // Split the current block after `spirv.mlir.loop`. The remaining ops will
diff --git a/mlir/test/Conversion/SPIRVToLLVM/control-flow-ops-to-llvm.mlir b/mlir/test/Conversion/SPIRVToLLVM/control-flow-ops-to-llvm.mlir
index 3557830e779e24..756fc5415e20f7 100644
--- a/mlir/test/Conversion/SPIRVToLLVM/control-flow-ops-to-llvm.mlir
+++ b/mlir/test/Conversion/SPIRVToLLVM/control-flow-ops-to-llvm.mlir
@@ -86,6 +86,14 @@ spirv.module Logical GLSL450 {
 //===----------------------------------------------------------------------===//
 
 spirv.module Logical GLSL450 {
+  // CHECK-LABEL: @empty_loop
+  spirv.func @empty_loop() "None" {
+    // CHECK: llvm.return
+    spirv.mlir.loop {
+    }
+    spirv.Return
+  }
+
   // CHECK-LABEL: @infinite_loop
   spirv.func @infinite_loop(%count : i32) -> () "None" {
     // CHECK:   llvm.br ^[[BB1:.*]]

@CoTinker CoTinker requested a review from joker-eph October 24, 2024 06:35
@CoTinker CoTinker merged commit 5aa741d into llvm:main Oct 26, 2024
11 checks passed
@CoTinker CoTinker deleted the empty_loop branch October 26, 2024 03:23
winner245 pushed a commit to winner245/llvm-project that referenced this pull request Oct 26, 2024
…lvm#113527)

This PR erases `spirv.mlir.loop` with an empty region in `LoopPattern`,
resolving a crash. Fixes llvm#113404.
NoumanAmir657 pushed a commit to NoumanAmir657/llvm-project that referenced this pull request Nov 4, 2024
…lvm#113527)

This PR erases `spirv.mlir.loop` with an empty region in `LoopPattern`,
resolving a crash. Fixes llvm#113404.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[mlir] -convert-spirv-to-llvm trigger assertion

3 participants